feat: initializeApp idempotency #2947

DellaBitta · 2025-07-02T16:58:50Z

Discussion

Update initializeApp to return an existing app if one exists with the same name and the same AppOptions. However, due to their inabilty to be compared, initializeApp will throw if an existing app has a configured Credential or httpAgent.

Additionally adds reference docs to getApp and getApps.

Testing

Updated unit tests.

API Changes

Internal: go/fb-admin-init-idempotency

Fixes: #2111

lahirumaramba

Overall LGTM! Added a few comments. Thank you!

lahirumaramba · 2025-07-15T16:25:55Z

src/app/lifecycle.ts

+ *   or `options.credential` are defined. This is due to the function's inability to
+ *   determine if the existing {@link App}'s `options` compare to the `options` parameter
+ *   of this function. It's recommended to use {@link getApp} or {@link getApps} if your
+ *   implementation uses either of these two fields in {@link AppOptions}.


Should we extend the JSDoc with an example?

/**... * For example, to safely initialize an app that may already exist: * * ```javascript * let app; * try { * app = getApp("myApp"); * } catch (error) { * app = initializeApp({ credential: myCredential }, "myApp"); * } * ``` */

lahirumaramba · 2025-07-15T16:43:40Z

src/app/lifecycle.ts

+      } else if (autoInit) {
+        // Auto-initialization is triggered when no options were passed to
+        // initializeApp. With no options to compare, simply return the App.
+        return currentApp;
      } else {


Can we simplify this by using early exits/guard clauses (and removing the nested if else statements)?
If it doesn't work in this case feel free to ignore. For example:

if (this.appStore.has(appName)) { const currentApp = this.appStore.get(appName)!; if (currentApp.autoInit() !== autoInit) { throw new FirebaseAppError( AppErrorCodes.INVALID_APP_OPTIONS, `Firebase app named "${appName}" attempted mismatch between custom AppOptions` + ' and an App created via Auto Init.' ) } if (autoInit) { return currentApp; } if (typeof options.httpAgent !== 'undefined') { .....

lahirumaramba · 2025-07-15T16:46:03Z

src/app/lifecycle.ts

+
+        // `httpAgent` objects cannot be compared with deepEquals impls.
+        // Idempotency cannot be supported if one exists.
+        if (typeof options.httpAgent !== 'undefined') {


nit: Would it make sense to move this (and below) validation to a separate function (out of initializeApp) to clean it up a bit?

lahirumaramba · 2025-07-15T16:49:20Z

src/app/lifecycle.ts

+ * @internal
+ */
+function validateAppNameFormat(appName: string): void {
+  if (typeof appName !== 'string' || appName === '') {


Can we use validator.isNonEmptyString() here?

firebase-admin-node/src/utils/validator.ts

Line 104 in 26b884f

export function isNonEmptyString(value: any): value is string {

lahirumaramba · 2025-07-15T16:49:59Z

src/app/lifecycle.ts

 export function initializeApp(options?: AppOptions, appName: string = DEFAULT_APP_NAME): App {
  return defaultAppStore.initializeApp(options, appName);
 }

+/**
+ * Returns an existing App instance for the provided name. If no name is provided
+ * the the default app name is queried.


queried or used?

lahirumaramba · 2025-07-15T16:50:55Z

src/app/lifecycle.ts

+ * A (read-only) array of all initialized apps.
+ * 
+ * @returns An array containing all initialized apps.
+ */


Let's get @egilmorez's review on the new doc changes, please. Thanks!

lahirumaramba · 2025-07-15T16:54:43Z

src/app/lifecycle.ts

-          'name.',
-        );
+          AppErrorCodes.INVALID_APP_OPTIONS,
+          `Firebase app named "${appName}" attempted mismatch between custom AppOptions` +


Can we simplify this error message? I am not sure Auto Init is a well known term in the Admin SDK world

egilmorez

Thanks David!

Some things to look at, most particularly literals. Professor Gilmorez urges you to review https://docs.google.com/presentation/d/1Ezb4sp4XD6ItHISccnME-EVH_oJj59R5Naq0IskLrAo/edit?slide=id.g1cd7eabb964_0_10#slide=id.g1cd7eabb964_0_10.

egilmorez · 2025-07-30T17:19:58Z

src/app/firebase-app.ts

+
+  /**
+   * Returns `true` if the FirebaseApp instance was initialized with a custom
+   * Credential.


I'd lowercase "credential" unless it's a literal.

And I would backtick FirebaseApp as a literal.

egilmorez · 2025-07-30T17:20:51Z

src/app/lifecycle.ts

      options = loadOptionsFromEnvVar();
      options.credential = getApplicationDefault();
    }

-    if (typeof appName !== 'string' || appName === '') {
+    // Check if an App already exists and, if so, ensure its AppOptions match


Couple of literals here too: "App already exists and, if so, ensure its AppOptions"

But maybe these comments don't get parsed for public docs; in that case, no big deal :)

egilmorez · 2025-07-30T17:22:29Z

src/app/lifecycle.ts

+ * Validates that the `requestedOptions` and the `existingApp` options objects
+ * do not have fields that would break deep equals comparisons.
+ * 
+ * @param requestedOptions The incoming AppOptions of a new initailizeApp


Do the literals in these @tags need to be backticked? I would guess so.

egilmorez · 2025-07-30T17:23:02Z

src/app/lifecycle.ts

 export const defaultAppStore = new AppStore();

+/**
+ * Initializes the App instance.


Either backtick the literal, or just call it an "app instance."

egilmorez · 2025-07-30T17:23:48Z

src/app/lifecycle.ts

+ * Note, due to the inablity to compare `http.Agent` objects and `Credential` objects,
+ * this function cannot support idempotency if either of `options.httpAgent` or
+ * `options.credential` are defined. When either is defined, subsequent invocations will 
+ * throw instead of returning an {@link App} object.


Should this be something like "throw an error," or cite the error name?

egilmorez · 2025-07-30T17:24:30Z

src/app/lifecycle.ts

+ *   the provided configuration.
+ *
+ * @throws FirebaseAppError if an {@link App} with the same name has already been
+ *   initialized with a different set of AppOptions.


Same question about the literals in these tagged descriptions:

@param options - Optional A set of {@link AppOptions} for the {@link App} instance.

If not present, initializeApp will try to initialize with the options from the

FIREBASE_CONFIG environment variable. If the environment variable contains a string

that starts with { it will be parsed as JSON, otherwise it will be assumed to be

pointing to a file.

@param appName - Optional name of the FirebaseApp instance.

@returns A new App instance, or the existing App if the instance already exists with

the provided configuration.

@throws FirebaseAppError if an {@link App} with the same name has already been

initialized with a different set of AppOptions.

@throws FirebaseAppError

egilmorez · 2025-07-30T17:25:00Z

src/app/lifecycle.ts

 export function initializeApp(options?: AppOptions, appName: string = DEFAULT_APP_NAME): App {
  return defaultAppStore.initializeApp(options, appName);
 }

+/**
+ * Returns an existing App instance for the provided name. If no name is provided


More questions about literals and backticks:

@param options - Optional A set of {@link AppOptions} for the {@link App} instance.

If not present, initializeApp will try to initialize with the options from the

FIREBASE_CONFIG environment variable. If the environment variable contains a string

that starts with { it will be parsed as JSON, otherwise it will be assumed to be

pointing to a file.

@param appName - Optional name of the FirebaseApp instance.

@returns A new App instance, or the existing App if the instance already exists with

the provided configuration.

@throws FirebaseAppError if an {@link App} with the same name has already been

initialized with a different set of AppOptions.

@throws FirebaseAppError

DellaBitta added 11 commits May 9, 2025 16:46

Initial implementation

f9e3b28

Merge branch 'master' into ddb-init-idempotency

aea60b7

Remove deep equals comparison

0c2d504

Added tests

14657e7

initial impl, needs testing, debug output removal.

018cb30

remove debug console logs and add comments

a471e5d

Unit tests

06189b7

Merge branch 'master' into ddb-init-idempotency-mk2

f13eda3

Revert accidental formatting changes

1d40a1c

API docs

dca658e

Documentation cleanup

9139e36

DellaBitta marked this pull request as ready for review July 10, 2025 15:07

DellaBitta requested a review from lahirumaramba July 10, 2025 15:07

DellaBitta changed the title ~~initializeApp idempotency (WIP candidate)~~ initializeApp idempotency Jul 10, 2025

comment cleanup.

c3ef699

lahirumaramba self-assigned this Jul 14, 2025

lahirumaramba added release-note release:stage Stage a release candidate labels Jul 14, 2025

lahirumaramba changed the title ~~initializeApp idempotency~~ feat: initializeApp idempotency Jul 15, 2025

lahirumaramba approved these changes Jul 15, 2025

View reviewed changes

Changes based on review feedback.

daa9eb6

lahirumaramba assigned egilmorez Jul 16, 2025

lahirumaramba requested a review from egilmorez July 16, 2025 14:50

replace deepEqual with fastDeepEqual

d33bca5

egilmorez reviewed Jul 30, 2025

View reviewed changes

DellaBitta added 2 commits July 30, 2025 15:20

Update docs with literal backticks, literally.

eadc2df

More internal literals.

ecb3d53

DellaBitta requested a review from egilmorez July 30, 2025 19:24

feat: initializeApp idempotency #2947

Are you sure you want to change the base?

feat: initializeApp idempotency #2947

Conversation

DellaBitta commented Jul 2, 2025 • edited by lahirumaramba Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Discussion

Testing

API Changes

Uh oh!

lahirumaramba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egilmorez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DellaBitta commented Jul 2, 2025 •

edited by lahirumaramba

Loading